fix(SDK-6463): survive glob/minimatch crash from incompatible brace-expansion (0-test builds)#1138
Merged
Bhargavi-BS merged 9 commits intoJul 2, 2026
Conversation
When a build requests accessibility (browserstack.json/browser caps) but the browserstackAccessibility plugin is not wired into the cypress config, the CLI now detects this before the build start event, explicitly disables accessibility so the build is not counted as an a11y build, warns the user with the setup doc link, and instruments the end-of-session EDS event (accessibility_plugin_not_loaded) so such builds can be excluded from stability queries. Detection is code-based: the plugin sets BROWSERSTACK_ACCESSIBILITY_PLUGIN_LOADED when its module is loaded/invoked by the cypress config; requireModule writes a flag file the parent process reads back. Falls back to a raw-source scan only when the config could not be required (e.g. TS before bstack packages are installed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-cli into disable-accessibility-when-plugin-not-loaded
…, not per test/hook [SDK-6574] The one-shot guard `if(!this.haveSentBuildUpdate && ...)` never flipped `haveSentBuildUpdate`, so the BuildUpdate event (a byte-identical, build-level `observability_version` payload) re-fired on every test/hook event — hundreds to thousands of identical events per build. Set the flag right after the event is sent so it is emitted once per reporter instance. Complements consumer-side dedupe TRAP-4033. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Memoize parsed cypress config in readCypressConfigFile (module-level cache keyed by resolved path) so the detection, capabilityHelper and video-config call sites share a single compile + child require per run. - Widen static binding detection to cover `import * as X` and bias the strict source-scan fallback toward keeping accessibility enabled when the plugin path is present but the binding can't be statically parsed (named/dynamic imports), avoiding silently disabling a billed feature. Still disables on positive evidence (path absent, or binding parsed and never called). - Guarantee cleanup of the detection flag file: delete any stale flag before the child runs, and unlink it in readCypressConfigFile's finally block. - Document the stripComments scrubber as intentionally best-effort/lossy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-plugin-not-loaded Disable accessibility when browserstackAccessibility plugin not loaded
…neshot-guard fix(observability): BuildUpdate one-shot guard — flip haveSentBuildUpdate [SDK-6574]
…nsion (SDK-6463) glob.sync (glob@7->minimatch@3, and even glob@10->minimatch@9) throws 'expand is not a function' / 'brace_expansion_1.default is not a function' when a project force-resolves brace-expansion to an incompatible major (e.g. 5.x) via yarn resolutions / npm overrides. That crash aborted getNumberOfSpecFiles and produced builds with 0 executed tests (or crashed the run). A secondary bug in deleteBaseUrlFromError (err.replace on a non-string) then masked the real error. - Wrap glob.sync in safeGlobSync: log once and return [] on failure so spec discovery and the run proceed (specs are resolved on BrowserStack regardless of the local count). - Backstop try/catch in getNumberOfSpecFiles so it never throws. - Guard deleteBaseUrlFromError against non-string errors. Verified against a real glob@7 + brace-expansion@5 crash. Adds regression tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9b8d34b
into
sdk-6463-nx-tsconfig-and-a11y-afterEach-fixes
5 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SDK-6463 (follow-up) — survive
glob/minimatchcrash from an incompatiblebrace-expansionA customer whose repo force-resolves
brace-expansion@5.0.5(a security bump, via yarnresolutions/ npmoverrides) hit a hard crash in spec discovery:Root cause (verified, reproduced)
The CLI depends on
glob@^7.2.0→minimatch@3, which importsbrace-expansionas a callable default (const expand = require('brace-expansion')).brace-expansion@5.xis ESM-only-shaped for CJS: it exports{ expand }with__esModule: trueand no callable default, soexpand(...)throws the moment a pattern contains braces ({js,ts,...}— which the Cypress default spec pattern always has).Note: simply upgrading
globdoes not fix this — I reproduced thatglob@10→minimatch@9also crashes withbrace-expansion@5.0.5(brace_expansion_1.default is not a function), becauseminimatch@9reads the (absent) default export too. So the robust fix is to make the CLI resilient to a broken transitive glob, not to chase a compatible glob version against a dependency the consumer pins.There was also a secondary bug that masked the real error:
deleteBaseUrlFromError(called from the run's error handler) doeserr.replace(...)on what may be a non-stringError, throwingTypeError: err.replace is not a function.Fix
safeGlobSyncwrapsglob.sync: on failure it logs one clear, actionable warning (points at thebrace-expansion/minimatchresolution) and returns[], so spec discovery and the run proceed (specs are resolved on BrowserStack regardless of the local count). All 4glob.syncsites inutils.jsnow use it.try/catcharoundgetNumberOfSpecFilesso it can never crash the run.deleteBaseUrlFromErrorguards non-string errors (typeof err === 'string' ? … : err) so the real failure is never masked.Impact
Before: builds created with 0 executed tests, or the run crashes outright. After: the run proceeds and BrowserStack executes the specs (local parallelisation may be reduced if the count couldn't be computed, but tests run).
Validation
glob@7/glob@10+brace-expansion@5.0.5→expand is not a function) and confirmedsafeGlobSyncdegrades it to[]with no crash.safeGlobSyncandgetNumberOfSpecFilesdon't throw whenglob.syncthrows;deleteBaseUrlFromErrorleaves non-string errors unchanged (and still transforms strings). Suite: +3 passing, no new failures.Scope
Separate branch/PR from the config-compile + accessibility-afterEach fixes (PR #1131), since this is a distinct root cause and can ship independently.
🤖 Generated with Claude Code